Skip to content

gh-141510: Update specializer to support frozendict#144949

Merged
corona10 merged 1 commit intopython:mainfrom
corona10:gh-141510-specializer-2
Feb 18, 2026
Merged

gh-141510: Update specializer to support frozendict#144949
corona10 merged 1 commit intopython:mainfrom
corona10:gh-141510-specializer-2

Conversation

@corona10
Copy link
Member

@corona10 corona10 commented Feb 18, 2026

@corona10
Copy link
Member Author

@Fidget-Spinner,
Since @markshannon prefers to use the current specializer pipeline (and I am also fine with this option), I opened a new PR.

@corona10 corona10 requested a review from vstinner February 18, 2026 12:47
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Your new PR optimizes two bytecodes instead of just one, and the change is smaller, nice!

In the future, we might use different opcodes for specialized frozendict operations to avoid locking, but we should first measure the performance benefits before considering going that far in terms in optimization. This change is a nice tradeoff between code complexity and performance.

@corona10 corona10 merged commit 3e2f5c1 into python:main Feb 18, 2026
78 checks passed
@corona10 corona10 deleted the gh-141510-specializer-2 branch February 18, 2026 16:11
@markshannon
Copy link
Member

@vstinner @corona10

This breaks STORE_SUBSCR_DICT as it allows stores to frozen dicts.
Please revert or fix.

To fix this you'll need to add a new guard for any dict. Use that for loads, and keep the old dict-only guard for stores.

And please add tests that specialized stores to frozen dicts fail.

@corona10
Copy link
Member Author

I will fix this today

@markshannon
Copy link
Member

markshannon commented Feb 20, 2026

BTW, it's quite tricky to write the test, you'll need a non-branching way to get a dict for specializing and a frozendict once specialized. This is a pattern we've used before:

dicts = [ {...}, frozendict(...) ]
...
for i in range(...):
    d = dicts[i == 100]
    d[...] = ...

Actually, that's only necessary for JIT tests, a simple d = frozendict() if i == 100 else {} should work as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Comments